Skip to content

🐛 server: accept bnb, matic, and mon activity assets - #1278

Closed
nfmelendez wants to merge 1 commit into
mainfrom
nicolas
Closed

🐛 server: accept bnb, matic, and mon activity assets#1278
nfmelendez wants to merge 1 commit into
mainfrom
nicolas

Conversation

@nfmelendez

@nfmelendez nfmelendez commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Chores
    • Updated project maintenance and security audit settings to support more consistent builds and dependency checks.
    • No changes to user-facing features or functionality.

@changeset-bot

changeset-bot Bot commented Sep 1, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: a0c54d5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: 835bc8f3-f232-459f-a37d-5d2e93a597b2

📥 Commits

Reviewing files that changed from the base of the PR and between c0d9b36 and a0c54d5.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (2)
  • package.json
  • pnpm-workspace.yaml

Walkthrough

The package configuration adds pnpm overrides for selected dependency versions. The workspace audit configuration adds one GitHub Security Advisory to its ignore list.

Changes

Dependency configuration

Layer / File(s) Summary
Dependency version overrides
package.json
pnpm now pins browserslist@4 to ^4.28.7 and adds overrides for nanoid@3 and postcss-selector-parser versions 6 and 7.
Audit advisory ignore list
pnpm-workspace.yaml
The audit configuration now ignores GHSA-vcc3-ghjq-m6fr.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to c0d9b

This localized change allows BNB, MATIC, and MON activity assets while adding coverage for the new behavior; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: cruzdanilo, mainqueg, dieguezguille

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: the server now accepts BNB, MATIC, and MON activity assets.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch nicolas
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch nicolas

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.04%. Comparing base (f76c1f1) to head (c0d9b36).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1278      +/-   ##
==========================================
+ Coverage   72.51%   73.04%   +0.53%     
==========================================
  Files         275      276       +1     
  Lines       13254    13257       +3     
  Branches     4610     4613       +3     
==========================================
+ Hits         9611     9684      +73     
+ Misses       3306     3236      -70     
  Partials      337      337              
Flag Coverage Δ
e2e 73.04% <ø> (+0.53%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nfmelendez

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c0d9b36b4a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/hooks/activity.ts Outdated
v.object({
category: v.picklist(["external", "internal"]),
asset: v.literal("ETH"),
asset: v.picklist(["BNB", "ETH", "MATIC", "MON"]),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Bypass the token allowlist for native assets

For BNB_MAINNET, MATIC_MAINNET, and MONAD_MAINNET deliveries, the missing contract address causes each newly accepted native asset to be normalized to the single Exa-chain WETH constant before isKnownToken checks the event chain's LI.FI token set. When that set is nonempty and does not contain the unrelated Exa WETH address, known resolves false and the funds-received notification is suppressed. The added test uses Anvil, where isKnownToken returns true unconditionally, so it cannot expose this production behavior; native assets should bypass the ERC-20 allowlist or use a chain-specific native/wrapped address.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a0c54d52ca

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread package.json
"abitype>zod": "^4.0.0",
"axios@1.16.1": "^1.18.0",
"brace-expansion@5.0.6": "^5.0.7",
"browserslist@4": "^4.28.7",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Restore the native-asset webhook changes

When Alchemy sends an external or internal transfer for BNB, MATIC, or MON, server/hooks/activity.ts:109-112 still accepts only asset: "ETH", so validatorHook rejects the event before account deployment, collateral processing, or notification. Fresh evidence relative to the earlier review is that this target has no server diff at all—its only changes are dependency and audit metadata—so the advertised native-asset fix has disappeared from the proposed commit.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant